Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Migration to Flutter 3.13 #4579

Merged
merged 8 commits into from
Sep 30, 2023
Merged

Conversation

g123k
Copy link
Collaborator

@g123k g123k commented Aug 21, 2023

Hi everyone,

Since the release is in a few days, we can already test if there is no regression with the latest stable version of Flutter.
In terms of code, there's no real breaking change.

@g123k g123k added the flutter label Aug 21, 2023
@g123k g123k self-assigned this Aug 21, 2023
@g123k g123k requested a review from a team as a code owner August 21, 2023 06:41
@github-actions github-actions bot added 🍎 iOS iOS specific issues or PRs 🥫 Product page 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… Android dependencies MacOS 🤳 ZXing labels Aug 21, 2023
Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have seen nothing shocking in that PR.
But I would wait about a month to see the first bug fixes, instead of rushing as soon as the first week of that new version's availability.
The new horizontal+vertical scrolling feature is not good enough a reason to do it, IMHO.

android {
compileSdkVersion 33
compileSdkVersion flutter.compileSdkVersion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're here you should probably change targetSdk too.

@g123k
Copy link
Collaborator Author

g123k commented Aug 21, 2023

I have seen nothing shocking in that PR. But I would wait about a month to see the first bug fixes, instead of rushing as soon as the first week of that new version's availability. The new horizontal+vertical scrolling feature is not good enough a reason to do it, IMHO.

There's also the fix for iOS, which is important (but also available on 3.10.7.
My point here is maybe to merge it, since we don't need to rush on the release and be pretty confident that the D day, there's no surprise

@monsieurtanuki
Copy link
Contributor

There's also the fix for iOS, which is important (but also available on 3.10.7. My point here is maybe to merge it, since we don't need to rush on the release and be pretty confident that the D day, there's no surprise

Upgrading to 3.10.7 would then be the wisest option. In the past other flutter developers (not us specifically) raised issues about new flutter versions and we quietly waited until most bug were fixed. I can see no good reason to rush this time.

@github-actions github-actions bot added the 📚 Documentation Improvements or additions to documentation label Aug 25, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2023

Codecov Report

Merging #4579 (53acc5c) into develop (53a75a3) will increase coverage by 9.91%.
Report is 2 commits behind head on develop.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #4579      +/-   ##
==========================================
+ Coverage         0   9.91%   +9.91%     
==========================================
  Files            0     310     +310     
  Lines            0   15792   +15792     
==========================================
+ Hits             0    1566    +1566     
- Misses           0   14226   +14226     
Files Coverage Δ
...b/bottom_sheets/smooth_draggable_bottom_sheet.dart 0.00% <ø> (ø)
...om_sheets/smooth_draggable_bottom_sheet_route.dart 0.00% <ø> (ø)
...app/lib/pages/product/ordered_nutrients_cache.dart 0.00% <ø> (ø)
packages/smooth_app/lib/themes/smooth_theme.dart 76.08% <ø> (ø)
...l/knowledge_panels/knowledge_panel_group_card.dart 0.00% <0.00%> (ø)
...l/knowledge_panels/knowledge_panel_table_card.dart 0.00% <0.00%> (ø)
...oth_app/lib/pages/onboarding/country_selector.dart 0.80% <0.00%> (ø)
...ib/pages/preferences/user_preferences_rate_us.dart 3.57% <0.00%> (ø)
...eferences/user_preferences_share_with_friends.dart 4.00% <0.00%> (ø)
packages/smooth_app/lib/main.dart 13.38% <0.00%> (ø)
... and 2 more

... and 298 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@teolemon
Copy link
Member

With a 3.13.1, I'd be tempted to merge anyway.

@monsieurtanuki
Copy link
Contributor

With a 3.13.1, I'd be tempted to merge anyway.

Sidenote: we may also have to check if the github action computers upgraded to 3.13.

@teolemon
Copy link
Member

Although I did merge @monsieurtanuki's simple bump, I'd like this bump to go through as well.
I haven't heard any issues on the general release, nor the dot one.

@g123k
Copy link
Collaborator Author

g123k commented Aug 27, 2023

Although I did merge @monsieurtanuki's simple bump, I'd like this bump to go through as well. I haven't heard any issues on the general release, nor the dot one.

I use it daily (on Android and iOS) and I haven't noticed any regression, so why not

@monsieurtanuki
Copy link
Contributor

For the record I'm not blocking anything here.

  • Do I find an added-value in the upgrade: no
  • Would I upgrade now: no
  • Would I find it catastrophic to upgrade now: no

@teolemon @g123k Don't be shy and upgrade if you want! 😉

Disclaimer: I don't know when the next release takes place.

@monsieurtanuki
Copy link
Contributor

@g123k They released 3.13.5 a week ago. I guess hope we're safe now!

@g123k
Copy link
Collaborator Author

g123k commented Sep 27, 2023

Unfortunately .5 is NOT a good release, because it breaks the fact to test on an iPhone (= a real device).
For now, I still use 3.13.4 for that.

A .6 was released a few minutes ago, I don't know if it fixes yet the regression

@g123k
Copy link
Collaborator Author

g123k commented Sep 29, 2023

Flutter 3.13.6 is still KO for iOS and Google has closed the bug -_-
So instead of being blocked by a hypothetical fix, I suggest migrating to 3.13.4

@monsieurtanuki
Copy link
Contributor

Flutter 3.13.6 is still KO for iOS and Google has closed the bug -_-

What about reopening the bug or creating a new one? We may not be the only project that needs to test on iphones?

@g123k
Copy link
Collaborator Author

g123k commented Sep 30, 2023

Flutter 3.13.6 is still KO for iOS and Google has closed the bug -_-

What about reopening the bug or creating a new one? We may not be the only project that needs to test on iphones?

I don't really understand, but I was trying to get the error message to post it here… and now with 3.13.6… it works.
So I will update the PR to this version

@monsieurtanuki
Copy link
Contributor

I don't really understand, but I was trying to get the error message to post it here… and now with 3.13.6… it works.

Let's hope you made a mistake when you tried initially and that actually there's no error at all.
Other possibility: non systematic error in 3.13.6. Less fun to deal with...

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we're ready then!

@g123k
Copy link
Collaborator Author

g123k commented Sep 30, 2023

Let's merge this 🚀

@g123k g123k merged commit c31e458 into openfoodfacts:develop Sep 30, 2023
6 checks passed
@g123k g123k deleted the flutter_313 branch September 30, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android dependencies 📚 Documentation Improvements or additions to documentation flutter 🍎 iOS iOS specific issues or PRs MacOS 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… 🥫 Product page 🤳 ZXing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Google Play store is mandating the app to target Android 13 (API level 33) Flutter 3.13 is available
4 participants